CI-sichere Release-Signing-Konfiguration für app und humanoperator#89
Conversation
There was a problem hiding this comment.
Review Summary
This PR implements CI-friendly release signing configuration for the app and humanoperator modules. The approach of using environment variables for signing credentials is sound, but critical issues must be addressed before merge:
Critical Issues (Must Fix)
-
Logic Error in both modules: Release build types unconditionally reference signing configs that may be uninitialized when environment variables are missing. The current guard logic doesn't protect against all build invocation scenarios (IDE builds, indirect task invocations), which could produce unsigned APKs without clear errors.
-
Security Vulnerability in both modules: Path traversal risk in keystore path handling - the
file()function accepts unvalidated paths from environment variables, enabling potential unauthorized file access on build systems.
Recommendation
Address the signing config conditional assignment and add path validation before merge. These defects could compromise build integrity and security in CI environments.
You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.
| signingConfigs { | ||
| create("release") { | ||
| if (missingReleaseSigningEnv.isEmpty()) { | ||
| storeFile = file(releaseSigningEnv.getValue("ANDROID_KEYSTORE_PATH")!!) |
There was a problem hiding this comment.
🛑 Security Vulnerability: Using file() with environment variable path enables path traversal attacks1. An attacker controlling ANDROID_KEYSTORE_PATH could read arbitrary files from the build system (e.g., ../../../../etc/passwd). Validate the path resolves within expected directories or use absolute path validation before passing to file().
Footnotes
-
CWE-22: Path Traversal - https://cwe.mitre.org/data/definitions/22.html ↩
| signingConfigs { | ||
| create("release") { | ||
| if (missingReleaseSigningEnv.isEmpty()) { | ||
| storeFile = file(releaseSigningEnv.getValue("ANDROID_KEYSTORE_PATH")!!) |
There was a problem hiding this comment.
🛑 Security Vulnerability: Using file() with environment variable path enables path traversal attacks1. An attacker controlling ANDROID_KEYSTORE_PATH could read arbitrary files from the build system (e.g., ../../../../etc/passwd). Validate the path resolves within expected directories or use absolute path validation before passing to file().
Footnotes
-
CWE-22: Path Traversal - https://cwe.mitre.org/data/definitions/22.html ↩
Co-authored-by: amazon-q-developer[bot] <208079219+amazon-q-developer[bot]@users.noreply.github.com>
Co-authored-by: amazon-q-developer[bot] <208079219+amazon-q-developer[bot]@users.noreply.github.com>
Motivation
google-services.jsonsoll unverändert versioniert bleiben und nicht Teil der Signing-Logik werden.Description
app/build.gradle.ktswerden die VariablenANDROID_KEYSTORE_PATH,ANDROID_KEY_ALIAS,ANDROID_KEYSTORE_PASSWORDundANDROID_KEY_PASSWORDausSystem.getenvgelesen und in einersigningConfigs { create("release") { ... } }verwendet.buildTypes.releaseist inappundhumanoperatorexplizit mitsigningConfig = signingConfigs.getByName("release")verknüpft und Nicht-Release-Builds bleiben unverändert.gradle.startParameter.taskNames) und löst bei fehlenden Variablen ein klareserror(...)mit den fehlenden Variablen aus.humanoperator/build.gradle.ktsergänzt und eine kurze Dokumentationdocs/ci-signing.mdangelegt;README.mdwurde um einen Link zur CI-Dokumentation ergänzt.Testing
./gradlew :app:help :humanoperator:helpwurde erfolgreich ausgeführt und bestätigt, dass die Gradle-Skripte syntaktisch auswertbar sind../gradlew :app:assembleRelease --dry-runund./gradlew :humanoperator:assembleRelease --dry-runwurden ausgeführt und liefern die erwartete, klare Guard-Fehlermeldung, wenn die Signing-Umgebungsvariablen nicht gesetzt sind../gradlew :app:lintDebug :humanoperator:lintDebugwurde versucht; Lint schlug fehl wegen einer bestehenden Kotlin-Metadata-Inkompatibilität in einer Abhängigkeit, die unabhängig von diesen Signing-Änderungen ist.Codex Task